Repository navigation
Conversation
github-actions
Bot
added
DABs
denik
force-pushed
the
denik/typed-config-walkers
branch
from
October 18, 2026 06:43
cb28c17 to
b955c4d
Compare
denik
added a commit
that referenced
this pull request
Oct 18, 2026
Co-authored-by: Isaac <no-reply@databricks.com>
Collaborator
Integration test reportCommit: f67c069
Top 6 slowest tests (at least 2 minutes):
|
arushiagarwal-db
pushed a commit
to arushiagarwal-db/cli
that referenced
this pull request
Oct 19, 2026
…atabricks#7085) ## Changes Twenty-one new `bundle validate` acceptance tests, each pinning current behaviour for a configuration edge case. They are grouped by topic under `acceptance/bundle/validate/` (`yaml_anchors/` for anchors and merge keys, `yaml_duplicate_keys/`, `variables/`, `targets/`, `includes/`, `nulls/`, `references/`); ten existing tests on the same topics moved into the groups. - merge keys and aliases in `targets.*.variables` (inherited through a target or root `<<`, aliases of a complex default, duplicate and malformed `<<`, inline mappings), - duplicate keys (with the processed JSON), timestamps as shorthands or nested in complex defaults, - references that resolve to the wrong type inside lists and maps, - includes overriding fields with `""` / `0` / `false`, diagnostics after a failed include merge, - `resources: null`, `targets: false`, `schedule: null`, - diagnostic locations for unknown fields in complex variables and duplicated app resources. `Badness` marks the cases where current behaviour is itself wrong: - a timestamp nested in a complex default fails with an internal error; a timestamp shorthand is dropped; - an inline mapping as the merge value is rejected although the YAML spec allows it; - duplicate keys are accepted silently (the later one wins); - an empty `resources:` section is an error without a location (empty `targets`, `variables`, `sync` are fine); `targets: false` fails with an internal "failed to rewrite" message; - an explicitly null `schedule` is reported as missing its required fields; - a failed include merge reports an error without a location, and hides the diagnostics of includes loaded after it. ## Why They guard behaviour ahead of databricks#7098, which replaces `libs/dyn`: where it differs, the test shows it. ## Tests Goldens recorded on main. This pull request and its description were written by Isaac. --------- Co-authored-by: Isaac <no-reply@databricks.com>
Contributor
|
Commit: Regression test reportSelected 20 changed tests (limit=20, 1 not selected) 42 tests: regression=22 unreleased=0 coverage=20 failing=0
TestAccept/bundle/python/pipeline-development-deprecated/DMS=/PYDAB_VERSION=0.266.0: branch ✅ | main (06660cf) ❌ | latest (v1.22.1) ❌main (06660cf): latest (v1.22.1): same output as main (06660cf) TestAccept/bundle/python/pipeline-development-deprecated/DMS=/PYDAB_VERSION=current: branch ✅ | main (06660cf) ❌ | latest (v1.22.1) ❌main (06660cf): latest (v1.22.1): TestAccept/bundle/python/pipeline-development-deprecated/DMS=true/PYDAB_VERSION=0.266.0: branch ✅ | main (06660cf) ❌ | latest (v1.22.1) ❌main (06660cf): latest (v1.22.1): same output as main (06660cf) TestAccept/bundle/python/pipeline-development-deprecated/DMS=true/PYDAB_VERSION=current: branch ✅ | main (06660cf) ❌ | latest (v1.22.1) ❌main (06660cf): latest (v1.22.1): same output as main (06660cf) TestAccept/bundle/refschema/DMS=: branch ✅ | main (06660cf) ❌ | latest (v1.22.1) ❌main (06660cf): latest (v1.22.1): TestAccept/bundle/refschema/DMS=true: branch ✅ | main (06660cf) ❌ | latest (v1.22.1) ❌main (06660cf): latest (v1.22.1): TestAccept/bundle/strict_profile_match/default_profile/host/DMS=: branch ✅ | main (06660cf) ❌ | latest (v1.22.1) ❌main (06660cf): latest (v1.22.1): same output as main (06660cf) TestAccept/bundle/strict_profile_match/default_profile/host/DMS=true: branch ✅ | main (06660cf) ❌ | latest (v1.22.1) ❌main (06660cf): latest (v1.22.1): TestAccept/bundle/strict_profile_match/host/DMS=: branch ✅ | main (06660cf) ❌ | latest (v1.22.1) ❌main (06660cf): latest (v1.22.1): same output as main (06660cf) TestAccept/bundle/strict_profile_match/host/DMS=true: branch ✅ | main (06660cf) ❌ | latest (v1.22.1) ❌main (06660cf): latest (v1.22.1): same output as main (06660cf) TestAccept/bundle/validate/app_resource_duplicate_name_location/DMS=: branch ✅ | main (06660cf) ❌ | latest (v1.22.1) ❌main (06660cf): latest (v1.22.1): same output as main (06660cf) 11 more failure details omitted, see the full report in the run. |
denik
force-pushed
the
denik/typed-config-walkers
branch
from
October 20, 2026 12:56
857e2d4 to
b9d8067
Compare
denik
added a commit
that referenced
this pull request
Oct 20, 2026
Co-authored-by: Isaac <no-reply@databricks.com>
ruslantalpa
pushed a commit
to ruslantalpa/cli
that referenced
this pull request
Oct 21, 2026
## Changes
Add `ForceSendFields` to `Presets`, `Lifecycle` and `Deployment`, so
explicit zero values (`false`, `0`, `""`) written in a target or include
replace non-zero base values.
Fixes on main: `targets.dev.resources.jobs.j.lifecycle.prevent_destroy:
false` was ignored and the base `true` stayed.
Visible change: `bundle validate -o json` now shows explicit zero
presets (e.g. `name_prefix: ""`) instead of `presets: {}`.
Not added to `Target`, `Workspace`, `Bundle`, `Experimental`, `Lock`,
`Artifact` or the other structs: they have pointer/map/slice fields, and
`ForceSendFields` there turns explicit `null` overrides (e.g. `run_as:`)
into forced values.
Prerequisite for removing `libs/dyn` (databricks#7098).
## Tests
- New `validate/lifecycle_override`: fails on main, passes here.
- `validate/includes/clears_unforced_preset_values` golden updated.
- Both pass on the databricks#7098 branch with this patch applied.
This pull request and its description were written by Isaac.
---------
Co-authored-by: Isaac <no-reply@databricks.com>
This was referenced Oct 21, 2026
denik
added a commit
that referenced
this pull request
Oct 21, 2026
Replace the generated pattern tables, matched against every node of the dynamic configuration with a trie, by generated walkers that descend only along the patterns' paths of the typed configuration. Null, zero, pure-reference values and locations still come from the dyn.Value at each node (walk_node.go), so the diagnostics are unchanged. Slice of #7098. Co-authored-by: Isaac <no-reply@databricks.com>
denik
added a commit
that referenced
this pull request
Oct 21, 2026
Replace the generated pattern tables, matched against every node of the dynamic configuration with a trie, by generated walkers that descend only along the patterns' paths of the typed configuration. Null, zero, pure-reference values and locations still come from the dyn.Value at each node (walk_node.go), so the diagnostics are unchanged. Slice of #7098. Co-authored-by: Isaac <no-reply@databricks.com>
denik
force-pushed
the
denik/typed-config-walkers
branch
from
October 21, 2026 15:50
d95e7b1 to
19dde68
Compare
denik
added a commit
that referenced
this pull request
Oct 21, 2026
Co-authored-by: Isaac <no-reply@databricks.com>
denik
added a commit
that referenced
this pull request
Oct 21, 2026
Replace the generated pattern tables, matched against every node of the dynamic configuration with a trie, by generated walkers that descend only along the patterns' paths of the typed configuration. Null, zero, pure-reference values and locations still come from the dyn.Value at each node (walk_node.go), so the diagnostics are unchanged. Slice of #7098. Co-authored-by: Isaac <no-reply@databricks.com>
denik
force-pushed
the
denik/typed-config-walkers
branch
from
October 21, 2026 16:03
19dde68 to
e82320b
Compare
denik
added a commit
that referenced
this pull request
Oct 21, 2026
Co-authored-by: Isaac <no-reply@databricks.com>
denik
added a commit
that referenced
this pull request
Oct 21, 2026
Replace the generated pattern tables, matched against every node of the dynamic configuration with a trie, by generated walkers that descend only along the patterns' paths of the typed configuration. Null, zero, pure-reference values and locations still come from the dyn.Value at each node (walk_node.go), so the diagnostics are unchanged. Slice of #7098. Co-authored-by: Isaac <no-reply@databricks.com>
denik
force-pushed
the
denik/typed-config-walkers
branch
from
October 21, 2026 16:12
e82320b to
4df1dd5
Compare
denik
added a commit
that referenced
this pull request
Oct 21, 2026
Co-authored-by: Isaac <no-reply@databricks.com>
…ong type Assign zeroed the slot, which left an empty string in lists and maps like main did not. Delete now removes slice elements, and resolution applies the updates from the last element to the first. Co-authored-by: Isaac <no-reply@databricks.com>
…e schedule warnings as Badness
…nerated code for the typed configuration Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
…as written Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
…uctvar; regenerate for the SDK update Co-authored-by: Isaac <no-reply@databricks.com>
…on; regenerate for the typed configuration Co-authored-by: Isaac <no-reply@databricks.com>
…typed Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
… a Badness the hybrid fixes Co-authored-by: Isaac <no-reply@databricks.com>
…sts of the load phase dynvar is replaced by structvar's identical reference helpers; merge keeps only Merge; pattern strings, tries and read-only walks are dropped. Unit tests that start from a typed configuration apply load-phase mutators via bundletest.ApplyLoading. Co-authored-by: Isaac <no-reply@databricks.com>
…lay; restore the Taskfile; regenerate
…he refschema golden for loctable
…eference; record the schedule warnings as Badness
…erence; use the renamed reference tests
… reference output
Co-authored-by: Isaac <no-reply@databricks.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
Every mutator used to convert the bundle configuration between a dynamic value tree (
libs/dyn) and the typedconfig.Rootand back, before and after it ran. On large bundles that copying dominated command time (about 93 ms per mutator on a 1000-job bundle, over 100 mutators inbundle validate).Now the configuration is loaded, includes are processed and the target is merged on the dynamic tree exactly as before. At the end of the load phase
Root.FinishLoadingconverts the tree once into typed structs and drops it. Everything after that, i.e. all of initialize, validate, plan and deploy, uses the structs only. There is no dyn ↔ struct conversion per mutator any more.structvar.DecodeDynis a third input of the typed decoder (next to YAML andView). It produces the struct, the pure${...}references of fields that cannot hold a string (e.g.max_retries: ${var.n}), and a tree of source locations, taken from the dynamic tree.Roothas two modes: dyn while loading (LoadDyn,Merge,MergeTargetOverrides,Mutate, as on main) and typed afterFinishLoading.libs/structs/structvarprovides a read-onlyView, writes, merge/override (used by the Python mutator,set_variables, JSON flags and the planner), and variable resolution with the semantics and error text ofdynvar. Where the whole configuration is traversed the code is generated bygo run ./bundle/internal/validation: required and enum walkers, resource defaults, a strings walker and resource iterators inconfig.generatecommands write YAML with the newlibs/structs/structyaml(identical output).libs/dynis unchanged here. The parts the load no longer uses are removed in #7203.ForceSendFields, never from locations. A value is present if it is non-zero, listed in its struct'sForceSendFields, or a pure reference; decoding records a written zero inForceSendFields. What this loses compared to the dynamic tree:ForceSendFieldsis indistinguishable from unset, e.g.value: 0in a job health rule orjob_id: 0inrun_job_taskwarns as not set;presets: {},resources: {}) is absent;source: ""onspark_python_task/sql_task.filegets theGITdefault on git_source jobs;developmentdeprecation warning is not shown fordevelopment: trueundermode: development(indistinguishable from the preset).workspace.hostas written, including a?o=or?a=query; on main it showed the host with the query removed. The success path is unchanged.Root.GetLocationOf/GetLocationsOfkeep main's names and return the closest location;...Exactvariants return where a value is written.GetLocation(path string)stays as on main.Why
bundle validateon a 1000-job bundle (test server; the machine was not quiet, median of 24 runs, interleaved):The one-time conversion costs about 64 ms and 85 MB at 1000 jobs.
Tests
Unit and acceptance tests. Because loading, includes and target merging are main's code again, the load/merge edge cases recorded in
acceptance/bundle/validate/{yaml_anchors,yaml_duplicate_keys,variables,targets,includes,nulls}(#7085) behave as on main again. The Badness markers that remain are main's own, plus: the location of an unknown field in a complex variable default is lost (raised by the typed resolver);presets.tagsare not applied to a job whosetagsis a reference to a whole map (same on main).Difference from #6954
#6954 deletes
libs/dynand reimplements loading and merging on structs. That cost behavior (merge-key inheritance, anchors, null versus empty in merges) and left traversals that use reflection slower than the dynamic tree for whole-tree walks. This PR keeps main's load and merge code, so those behaviors are unchanged, and uses generated walkers in the typed phase.This pull request and its description were written by Isaac.